testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py (1)

69-73: TODO: assert the expected validation message once available.

Until the message is asserted, this test only checks Accepted=False, which could pass for an unrelated rejection reason. Tighten it when the upstream validation lands.

Would you like me to open a tracking issue for adding the message assertion here?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@testsuite/tests/singlecluster/extensions/pipeline_policy/test_pipeline_policy_validation.py`
around lines 69 - 73, Test currently only asserts
policy.wait_until(has_condition("Accepted", "False"), ...) which can match
unrelated rejections; update the test to also assert the expected validation
message once the upstream validation is implemented by replacing the TODO with
an assertion that the policy's condition contains the exact expected message
(use policy.refresh().model.status.conditions to read conditions) — for example,
extend has_condition usage or add an assert that any(c.type == "Accepted" and
c.status == "False" and expected_message in c.message for c in
policy.refresh().model.status.conditions) where expected_message is the upstream
validation text; leave a TODO/issue reference if the message is not yet
available.

testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py (1)

8-22: ⚖️ Poor tradeoff

Optional: consider centralising the duplicated pipeline_policy/commit fixtures.

The header-only pipeline_policy fixture and the commit autouse fixture are repeated (with small variations in committed components) across the three interaction modules. A shared parametrised helper in interactions/conftest.py could reduce drift, though the per-module differences make this a low priority.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@testsuite/tests/singlecluster/extensions/pipeline_policy/interactions/test_pipeline_policy_ratelimit.py`
around lines 8 - 22, Extract the duplicated pipeline_policy fixture and the
autouse commit fixture into a shared interactions/conftest.py: move the
header-only pipeline_policy setup (the pipeline_policy fixture that calls
pipeline_policy.on_http_response.add_headers([...]) and returns pipeline_policy)
and the commit fixture (which iterates components, calls
request.addfinalizer(component.delete), component.commit(), and
component.wait_for_ready()) into conftest.py, and make commit parametrisable so
callers can pass which components to commit (e.g., via pytest param or an
injected fixture) while preserving autouse behavior where needed; update the
three interaction test modules to import/rely on the shared pipeline_policy and
to call the commit fixture with the appropriate component list to avoid
duplication.

testsuite/kuadrant/extensions/pipeline_policy.py (1)

77-83: 💤 Low value

Inconsistent header serialisation between add_headers and add_deny.

add_headers serialises the headers list via str(headers), producing a Python repr with single quotes (e.g. [['x-single', 'one']]), whereas add_deny(with_headers=...) expects a caller-supplied double-quoted string (e.g. '[["x-deny-reason", "blocked"]]'). This only works if the extension parses headersToAdd/withHeaders as CEL (single quotes valid) rather than strict JSON. Consider accepting a List[List[str]] in both methods and serialising consistently to avoid surprises for future callers.

Please confirm the extension accepts a Python-style single-quoted list for headersToAdd (i.e. CEL evaluation rather than JSON parsing); the response-header tests should fail if it does not.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@testsuite/kuadrant/extensions/pipeline_policy.py` around lines 77 - 83, The
add_headers method currently serialises headers with Python's str(headers)
producing single quotes, causing inconsistency with add_deny's expected
double-quoted JSON; change add_headers (and ensure add_deny) to accept headers
as a List[List[str]] and serialise them with json.dumps before placing into the
action dict (keys "headersToAdd" and "withHeaders"), preserving predicate
handling and the `@modify` decorator so both methods produce consistent JSON-style
double-quoted strings for downstream parsing.